Skip to content

Conversation

@armenzg
Copy link
Member

@armenzg armenzg commented Jan 6, 2026

This change adds support for forwarding GitHub pull_request events to Seer instead of Overwatch.

@armenzg armenzg self-assigned this Jan 6, 2026
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jan 6, 2026
WEBHOOK_EVENT_PROCESSORS = (_handle_pr_webhook_for_autofix_processor,)
WEBHOOK_EVENT_PROCESSORS = (
_handle_pr_webhook_for_autofix_processor,
code_review_handle_webhook_event,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this to the PullRequestEventWebhook class which handles pull_request events.

GithubWebhookType.PULL_REQUEST: PullRequestEventWebhook,

PULL_REQUEST = "pull_request"

sample_rate=1.0,
tags={
"organization_id": organization.id,
"repository_id": repo.id,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you run the tests with verbosity, you can see that these calls to metrics.incr are failing because these two keys are forbidden due to high cardinality.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajay-sentry @srest2021 heads up ^ in case you were still using that metric to track billing related stuff for GA

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this!!

- Sending webhook events with proper authentication
"""

OPTIONS_TO_SET: Mapping[str, Any] = {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New approach to initialized the test classes. The class needs to define which options to set as part of the set up.

Image Image

Copy link
Member

@suejung-sentry suejung-sentry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is great and much simpler than I expected it to be! Nice!


EVENT_TYPE = IntegrationWebhookEventType.MERGE_REQUEST
WEBHOOK_EVENT_PROCESSORS = (_handle_pr_webhook_for_autofix_processor,)
WEBHOOK_EVENT_PROCESSORS = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for my own mental model, these 2 processors run serially right?

for processor in self.WEBHOOK_EVENT_PROCESSORS:

With a failure in 1 does not block the next, and each processor runs synchronous for most of its logic except for the call to seer in schedule_task which is async?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is correct.

Once the code review processors executes, the work is in the task broker queue and will be handled async.

sample_rate=1.0,
tags={
"organization_id": organization.id,
"repository_id": repo.id,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajay-sentry @srest2021 heads up ^ in case you were still using that metric to track billing related stuff for GA

def _get_trigger_for_action(action: PullRequestAction) -> CodeReviewTrigger:
if action == PullRequestAction.READY_FOR_REVIEW:
return CodeReviewTrigger.ON_READY_FOR_REVIEW
return CodeReviewTrigger.ON_NEW_COMMIT
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically this could be something like below so it's not a burden of the caller of the helper to know which actions are actually valid

match action:
    case PullRequestAction.READY_FOR_REVIEW:
        return CodeReviewTrigger.ON_READY_FOR_REVIEW
    case PullRequestAction.SYNCHRONIZE:
        return CodeReviewTrigger.ON_NEW_COMMIT
    case _:
        raise ValueError(f"Unsupported pull request action: {action}")

if action not in WHITELISTED_ACTIONS:
return

if pull_request.get("draft") is True:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later I'm guessing we're going to want to start tracking this metric later too to inform whether we should allow our feature on drafts or not

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not certain if this is necessary since we check the actions and there's a draft action.

"""Helper to set up code review test context."""
self.organization.update_option("sentry:enable_pr_review_test_generation", True)
features_to_enable = self.CODE_REVIEW_FEATURES if features is None else features
options_to_set = dict(self.OPTIONS_TO_SET) | (options or {})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is nice

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Support sending GitHub `pull_request` events to Seer instead of Overwatch.
@armenzg armenzg marked this pull request as ready for review January 7, 2026 18:54
@armenzg armenzg requested a review from a team as a code owner January 7, 2026 18:54
@armenzg armenzg enabled auto-merge (squash) January 7, 2026 18:56
@armenzg armenzg merged commit 91e9e98 into master Jan 7, 2026
67 checks passed
@armenzg armenzg deleted the 1_5/code_review/pull_request/armenzg branch January 7, 2026 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants